feat: list repositories and cd to one by name - #13

Verdict: changes requested

Reviewed at the pushed head, commits bdbe80b..f93d885. task check passes: fmt, vet, golangci-lint, markdownlint, yamllint, actionlint, goreleaser check, gitleaks, go test -race across all packages, build, all green.

Acceptance criteria from issue #3, checked one by one against the real binary, not just the diff:

• scan walks root into Repo values and stops at a boundary, with a worktree test: met for a worktree that sits beside its main checkout, not met for one nested inside it. See Blocker below.
• list prints every repository under root: met for the topologies tested (plain repo, nested vendored repo correctly hidden, sibling worktree hidden by default). Incomplete for the nested case, same Blocker.
• list --json: met, verified structurally and via TestRunList_JSONReportsTheSameRepositoriesStructured.
• list --worktrees includes a linked worktree: met for a sibling worktree, verified directly (list showed 4 paths, list --worktrees added the fifth). Not met for a worktree nested inside its own repository's working tree.
• cd <query> resolves a unique partial match on stdout, exit 0: met, verified directly (cd widg -> resolved path, exit 0; cd foo -> exit 0).
• Ambiguous query with a terminal shows a numbered picker on stderr: met, verified via TestRunCd_AmbiguousQueryWithATerminalReadsAPickFromStdin and by reading pick().
• Ambiguous query without a terminal exits non-zero and lists candidates on stderr: met, verified directly through a real pipe (printf '' | gitcd cd acme), exit 1, both candidates listed.
• Query matching nothing exits non-zero naming the query: met, verified directly (cd doesnotexistatall -> exit 1, message names the query).
• Shell completion for cd in zsh and fish through shell-init: met. Ran the real zsh completion function in a real zsh process with compadd/compdef stubbed and a fake gitcd on PATH; it correctly reduced two full paths to their short names. Fish stays a string assertion since fish is not installed here, consistent with the acceptance criterion and the repository's own constraints.
• Tests cover matching, ambiguity and the non-terminal path: met, and two of the three guards I reverted in a detached scratch copy genuinely failed without their code (see Tests below).
• task check passes: met, see above.

Boundary probe (built a real tree under $TMPDIR: a plain repository, a repository with a sibling linked worktree, a repository with a nested repository in vendor/, and a repository whose own .claude/worktrees/<branch> holds a linked worktree, matching this very project's own worktree convention):

• Plain repository: found by list. Correct.
• Nested repository inside another repository's working tree (outer/vendor/innerlib): never reported, by list or list --worktrees. Correct, no double reporting.
• Sibling linked worktree (widget / widget-feature): hidden by list, shown by list --worktrees. Correct, matches the acceptance criterion.
• Linked worktree nested inside its own repository's working tree, at <repo>/.claude/worktrees/<branch>: invisible to both list and list --worktrees, and cd <branch> fails with "no repository matches". This is the exact shape this organization's own worktree convention produces, and the one the issue text names explicitly ("covers linked worktrees and .claude/worktrees/ in one rule"). It does not cover it; see Blocker.

Ranking attack probes (built a second tree: a repository named git whose name is a literal substring of another repository gitcd's path; a repository widget/api whose owner equals another repository's exact name widget; owners shared by several repositories with no repository literally named acme):

• cd git resolved only acme/git, not acme/gitcd. Exact name won outright, no bleed-through.
• cd gitcd resolved only acme/gitcd.
• cd widget resolved only acme/widget, not widget/api, even though widget/api's rel path also starts with "widget". Exact name won outright, as documented.
• cd acme (an owner, not a repository name) fell through to the ambiguous picker across all five repositories under that owner, as expected; no owner ever won as if it were a repository.

No break found in matching or ranking. The claim in the pull request description holds under all four probes.

Exit codes measured directly, not just asserted in tests:

• list: 0.
• cd with a unique match: 0.
• cd with no match: 1.
• cd ambiguous through a real pipe (not a terminal): 1, candidates listed on stderr.
• cd ambiguous with a terminal and a valid pick (via the injected isTerminal test path): 0.
• cd ambiguous with a terminal and an invalid pick: 1.
  All match the acceptance criteria and the pull request's own table.

Terminal detection limitation: confirmed directly. cd acme < /dev/null and cd acme < /dev/zero (the latter bounded, it returns in well under a second, not a hang) both get misread as a terminal, print the numbered picker, then fail with "no selection was read for ...", exit 1. Functionally this still satisfies the acceptance criterion: non-zero exit, candidates already listed on stderr before the failure. The only problem is the stated reason being wrong. A script that redirects stdin from /dev/null to guarantee non-interactive behavior gets the right exit code for the right practical outcome, with a confusing message. I judge this a Should, not a Blocker, as scoped in the Risk section.

Tests, revert-and-fail, in a detached copy of the source tree only (never the worktree, since its .git is a file pointing at the shared git dir), removed afterward:

• internal/git.IsRepository required the .git entry to be a directory (reverting the file-or-directory boundary rule): TestWalk_ReportsALinkedWorktreeWithoutDescendingIntoIt failed, lost the worktree from the result. Guard is real.
• Removed the !isTerminal check in cd.go's pick(): TestRunCd_AmbiguousQueryWithoutATerminalFailsListingEveryCandidate failed (exit 0 instead of the expected failure). This is the test the author reported fixing after discovering it passed for the wrong reason with the guard deleted, because stdin was an already-closed pipe. Confirmed: the fixed version now fails for the right reason when the guard is gone.
• Removed the exact-match tier in resolveQuery, falling back to substring-only matching: TestRunCd_ExactNameWinsOverABroaderFuzzyMatch failed, fell into the ambiguous picker instead of resolving outright. Guard is real.

All three reverts were restored before moving on; the scratch copy was deleted.

Isolation: ran every probe with HOME under $TMPDIR and GIT_CONFIG_NOSYSTEM=1. Checked the real /Users/simonostendorf/.gitconfig by its absolute path without overriding HOME: 537 bytes, mtime Oct 17 08:38:06 2026, no occurrence of the string gitcd. Unchanged throughout.

Blocker

• internal/scan/scan.go:79 (and the boundary comment at internal/scan/scan.go:76-79): a repository boundary's SkipDir stops Walk from ever looking inside that repository's own working tree, which is also where this organization's own worktree convention places linked worktrees, under <repo>/.claude/worktrees/<branch>. Issue feat: list and cd with fuzzy matching #3 states the boundary rule is meant to cover "linked worktrees and .claude/worktrees/ in one rule rather than two special cases"; it only covers a worktree that sits beside its main checkout as a sibling path, the one shape internal/scan/scan_test.go:142-183 tests. A worktree nested inside its own repository's working tree, reproduced directly under $TMPDIR with the exact relative layout this repository's own worktrees use, is invisible to list --worktrees and unreachable by cd, which fails with "no repository matches". Fix: have Walk (or profile.Inspect) also surface a repository's registered linked worktrees directly, for instance from git -C <path> worktree list --porcelain, rather than relying on physically walking into a path that the boundary rule always skips past. Add a test that nests a linked worktree inside its own main checkout's working tree, not only beside it, since that is the layout actually in use.

Should

• cmd/gitcd/cd.go:196-202 (isTerminalFile): confirmed that /dev/null and /dev/zero are misread as a terminal, producing "no selection was read for %q" instead of the correct "stdin is not a terminal" message, though the exit code and the candidate listing are still correct. Worth a one-line note in isTerminalFile's doc comment pointing at this, or a narrow check for these two well-known device files, next time this code is touched; not worth blocking on since the practical behavior for a script is already correct.